pkp/pkp-lib#11327 Implement public facing UI for user comments #659
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
taslangraham
force-pushed
the
i11327-main
branch
3 times, most recently
from
2f3351f to
1f2de49
Compare
taslangraham
changed the title
taslangraham
force-pushed
the
i11327-main
branch
from
1f2de49 to
c528830
Compare
There was a problem hiding this comment.
Thanks @taslangraham. I'll let @jardakotesovec comment on anything specific about the Vue implementations, but it overall looks good to me. Just a few questions.
| @@ -0,0 +1,108 @@ | |||
| <template> | |||
| <PkpModalBody> | |||
There was a problem hiding this comment.
The close button isn't highlighting and being treated as a clickable element by the mouse. It's still keyboard navigable, but it's not receiving the typical visual feedback from the pointer.
There was a problem hiding this comment.
To give you heads up. After we merge this version.. I will start gradually refining some of the components there.. and possibly reorganise things once I have good styling strategy settled. I would probably make these adjustments myself to see it in practice and just ask you for review. Lets see how that goes.
src/frontend/components/PkpAvatarInitials/pkpAvatarInitials.vue
Outdated
Show resolved
Hide resolved
| default: false, | ||
| }, | ||
| /** Use when this button represents an action such as delete, go back, revert or cancel. */ | ||
| isWarnable: Boolean, |
There was a problem hiding this comment.
I will confirm this one with Devika.. maybe we can have just primary and secondary. Goal here will be again just have as least variants as possible.. so its easier to create styling across the themes.
There was a problem hiding this comment.
Any updates on this?
There was a problem hiding this comment.
Most likely we will be fine just using the primary&secondary styling. So lets stick with that for now. Primary would be used for delete button, and secondary for cancel.
| pageCount.value = pagination.value.pageCount; | ||
| showMoreCommentsCount.value = | ||
| showMoreCommentsCount.value - props.itemsPerPage; | ||
| } |
There was a problem hiding this comment.
Looks good.. useFetchPaginated was not really created for "show more".. its done for using pagination.. so there would be opportunity to abstract that better.. but I think this is fine.
taslangraham
force-pushed
the
i11327-main
branch
5 times, most recently
from
67ae0c2 to
2b21f3f
Compare
|
Thanks for the review @ewhanson & @jardakotesovec. @jardakotesovec I've made update based on feedback given. Please take a look. |
taslangraham
force-pushed
the
i11327-main
branch
2 times, most recently
from
39adda2 to
4fc9ba7
Compare
taslangraham
force-pushed
the
i11327-main
branch
from
4fc9ba7 to
7459371
Compare
For pkp/pkp-lib#11327